Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolving issues link checker #486

Merged
merged 21 commits into from
Apr 29, 2024

Conversation

cofinoa
Copy link
Collaborator

@cofinoa cofinoa commented Apr 24, 2024

No description provided.

@cofinoa cofinoa marked this pull request as ready for review April 24, 2024 17:59
@cofinoa cofinoa self-assigned this Apr 24, 2024
@cofinoa
Copy link
Collaborator Author

cofinoa commented Apr 24, 2024

Dear @cf-convention/info-mgmt team, this PR relates to long-standing issue with the link checker, see: #318 #320

This is a first step to fix the issues with the link checker when PR are made.

The action is triggered when PR are open/re-open and

  1. first check if all **/*.md files in the repo has no broken links, if so the action fails, in the summary of the action run, the resulting output can be seen, including when "everything" is "green". From https://github.com/cf-convention/cf-convention.github.io/actions/runs/8821086583?pr=486 :

Summary

Status Count
🔍 Total 593
✅ Successful 495
⏳ Timeouts 0
🔀 Redirected 0
👻 Excluded 98
❓ Unknown 0
🚫 Errors 0
  • I have had to fix some existing links to the rendered .html files, and change them to the actual .md which is what should be referenced in .md files instead, see commit 468dece
  • the change at vocabularies.md has been tricky because the md file uses HTML tags.
  1. If .md are ok, the action checks that building the site with Jekyll works and, if so, it uploads the artifact.

Please take a moment to review and let me know if this fits. If so, I will continue with the PR to incorporate the link check of the site at regular basis (i.e. cron job every Monday), or just we can merge this PR and open a new one PR for the that.

@cofinoa
Copy link
Collaborator Author

cofinoa commented Apr 24, 2024

PS: annotation of the PR with a comment with the link check report it's a challenge due to security issues with PR from forks. If PR are from same repo (not) forks then PR and ISSUE commenting it's possible.

PS2: Checking links to GITHUB may raise and issue with the limit rate of GITHUB HTTP requests

@larsbarring
Copy link
Contributor

Hi Antonio,

  • the change at vocabularies.md has been tricky because the md file uses HTML tags.

I was recently adding some minor changes to this file, and noticed that there is actually very little markdown, and a lot of repetitive html links. I though that it maybe would be possible to generate this file dynamically during the build process. Something like a small [python] script looking for through the relevant ../Data/ directories for which versions exists and then assembles the file based on that and md text fragments, either read from file(s) or stored within the script. Could this be something to look further into (I'm afraid it's beyond my skill set)?

@cofinoa cofinoa mentioned this pull request Apr 25, 2024
@cofinoa
Copy link
Collaborator Author

cofinoa commented Apr 25, 2024

@larsbarring I have made a new PR at #487 with your suggestion to refactor vocabularies.md

@JonathanGregory JonathanGregory mentioned this pull request Apr 25, 2024
@JonathanGregory
Copy link
Contributor

Dear Antonio @cofinoa

Your work on this is very useful and much appreciated. Thanks! I would like to suggest that it makes the history of what's happened easier to follow if we keep to the practice of putting substantial comments in the issue, rather than PRs. That's because the issue is a continuous record of activity, whereas there can be lots of PRs, including comments on documents that aren't arranged serially. So I have copied the above comments from you and Lars into #318.

Best wishes

Jonathan

@cofinoa cofinoa merged commit d4a0ff5 into cf-convention:main Apr 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants